fix: demo startup hardening, release acceptance coverage, and safe ADK multi-agent delegation - #111
Merged
Merged
Conversation
Two demo commands broke while the workflow was still coming up. ``app.workflow`` raises until the controller reports READY, so ``/memory`` and ``/kb-backfill`` surfaced the generic "Error executing command: Workflow not initialized yet" during background init — which reads like a bug rather than "not yet". Both now warn and return, using the same readiness shape the built-in commands use. ``ResearchDemoSettings``' documented precedence was also wrong: it omitted constructor arguments, named the wrong project path, and did not say that the project file is untrusted and allowlist-filtered. The gap this closes on the test side is application *composition*. The workflow, the renderer and the individual commands all had component tests; nothing exercised a real ``ResearchDemoApp`` reacting to real input, and nothing exercised the thing a user actually runs. - Headless composition tests drive the real app through ``BaseCLIApp.process_input`` and the real ``MessageProcessor``, substituting only the nondeterministic workflow (scripted ``WorkflowEvent`` streams) and the UI (``RecordingSession``). They cover readiness warnings, message routing and session-id propagation, unknown commands, and recovery after a failed turn. Reverting the command fix turns two of them red with the exact reported error. - A pty smoke drives the real console process: startup → prompt → /help → /exit, asserting no traceback and exit 0, with no API key, network, Docker or LLM. It also pins that the run is side-effect free and that the child resolves its modules from the checkout. - A built-wheel acceptance builds the wheel, installs it into a throwaway virtualenv and runs the same session through the installed ``research-demo`` console script, asserting the child resolves inside the venv and that the package data (benchmarks.csv, the report-writer SKILL.md, report_template.tex) shipped. It is opt-in (``wheel`` marker + ``AGENTIC_WHEEL_ACCEPTANCE=1``) and runs as its own CI job, so the offline suite stays fast and network-free. ``tests/demo_isolation.py`` exists because ``ResearchDemoSettings`` reads three developer-owned sources that are *not* isolated alike: the two JSON files follow ``cwd``/``HOME`` at call time, but ``model_config["env_file"]`` is frozen at class definition — i.e. at collection, before any fixture runs. Redirecting HOME never moved it, so headless tests were reading the developer's real ``~/.research_demo/.env``. An explicit ``_env_file`` is the only thing that moves it, and the effective path is asserted rather than assumed. pexpect is declared in the dev extra rather than relied on transitively. Claude-Session: https://claude.ai/code/session_01BpktWb9vKN5exdT9MbLQYh
Two defects made ADK's built-in agent hand-off unusable, so any demo or app with ``sub_agents`` could not delegate at all. **The routing tool was denied as unregistered.** ``PermissionPlugin`` unwraps ``.func`` only for exact ADK function-tool types, because a subclass may override ``run_async`` and run something other than the callable it advertises. ``TransferToAgentTool`` is a ``FunctionTool`` *subclass* that ADK auto-injects, so it resolved to no registry identity and was refused — even with permissions disabled, since the refusal happens before the engine is consulted. Its exact class now joins the trusted types, on the same terms and for the same reason they are trusted: ADK constructs it as ``super().__init__(func=transfer_to_agent)`` and overrides only ``_get_declaration()`` (to add the agent-name enum), never ``run_async``, so what it invokes is still exactly ``self.func``. Listing the exact class keeps every other subclass out; no name-based authority was reintroduced and no alias was added. **The tool told the model to call something that does not exist.** ADK builds the declaration from ``transfer_to_agent``'s docstring, which through 1.37.0 — the newest release inside our ``<2`` pin, checked against the published wheel — advises callers to "use TransferToAgentTool instead of this function directly". That paragraph is written for Python callers but ships to the model as the tool's description, and Gemini 3.1 followed it, emitting ``TransferToAgentTool`` for ADK to reject with ``Tool 'TransferToAgentTool' not found``. A narrowly scoped before-model plugin rewrites that description on the prepared request. It acts only when the tool object is exactly ``TransferToAgentTool`` (an application tool sharing the name is untouched) and only while the misleading sentence is present, so it is idempotent and becomes a no-op the day the installed ADK ships a corrected docstring — upstream fixed it in 2.x. Only ``description`` is written: the declaration name, parameter schema, required fields and the agent-name enum are preserved, and the upstream function's ``__doc__`` is never mutated. Regression coverage asserts both halves, including that widening the trusted type list opened no hole: an arbitrary ``FunctionTool`` subclass, a ``TransferToAgentTool`` subclass, a forged object named ``transfer_to_agent``, one named after the class, and one carrying a copied ``.func`` all stay denied. The declaration tests drive a synthetic misleading declaration rather than the installed one, so they do not require upstream to remain broken; a single integration test accepts either state and asserts the outcome is safe. The demo's coordinator prompt gains a matching policy: an explicit bounded request is executed or delegated immediately, an open-ended goal (or an explicit request for a plan) is planned and held for confirmation, and planning is stated to be a workflow courtesy rather than the authorization boundary — tool permissions remain responsible for that. The live scenarios are split into three independent contracts (planning, bounded delegation, KB ingest/readback) over a persistent conversation, so one stochastic policy choice can no longer fail all three. Claude-Session: https://claude.ai/code/session_01BpktWb9vKN5exdT9MbLQYh
shoom1
force-pushed
the
test/pre-release-acceptance-0.6.0
branch
from
August 4, 2026 02:51
4e6e2c8 to
d3d5500
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pre-release work for 0.6.0: acceptance coverage for the thing users actually run, and two verified correctness defects in ADK multi-agent delegation.
Two commits, separable and each independently green.
1.
fix(demo): harden startup and add release acceptance coverage(0e4662a)/memoryand/kb-backfilltouchedapp.workflowduring background initialization, where it raises — surfacing the genericError executing command: Workflow not initialized yet, which reads like a bug rather than "not yet". Both now warn and return, using the readiness shape the built-in commands already use.ResearchDemoSettings' precedence documentation was also wrong: it omitted constructor arguments, named the wrong project path, and did not mention that the project file is untrusted and allowlist-filtered.The coverage gap this closes is application composition. The workflow, renderer and individual commands all had component tests; nothing exercised a real
ResearchDemoAppreacting to real input, and nothing exercised the shipped console at all.BaseCLIApp.process_inputand the realMessageProcessor, substituting only the nondeterministic workflow (scriptedWorkflowEventstreams) and the UI (RecordingSession). Covers readiness warnings, message routing, session-id propagation, unknown commands, and recovery after a failed turn. Reverting the command fix turns two of them red with the exact reported error./help→/exit, asserting no traceback and exit 0, with no API key, network, Docker or LLM. Also pins that the session is side-effect free and that the spawned child resolves its modules from the checkout.research-democonsole script, asserting the child resolves inside the venv (not the checkout) and that package data shipped. Opt-in (wheelmarker +AGENTIC_WHEEL_ACCEPTANCE=1) and run as its own CI job, so the offline suite stays fast and network-free.tests/demo_isolation.pyexists becauseResearchDemoSettingsreads three developer-owned sources that are not isolated alike: the two JSON files followcwd/HOMEat call time, butmodel_config["env_file"]is frozen at class definition — i.e. at collection, before any fixture runs. RedirectingHOMEnever moved it, so headless tests were reading the developer's real~/.research_demo/.env. An explicit_env_fileis the only thing that moves it, and the effective path is asserted rather than assumed.pexpectis declared in thedevextra rather than relied on transitively.2.
fix(adk): restore safe multi-agent delegation(d3d5500)Two defects made ADK's built-in agent hand-off unusable, so any app with
sub_agentscould not delegate.The routing tool was denied as unregistered.
PermissionPluginunwraps.funconly for exact ADK function-tool types, because a subclass may overriderun_asyncand run something other than the callable it advertises.TransferToAgentToolis aFunctionToolsubclass that ADK auto-injects, so it resolved to no registry identity and was refused — even with permissions disabled, since the refusal precedes the engine. Its exact class now joins the trusted types, on the same terms: ADK constructs it assuper().__init__(func=transfer_to_agent)and overrides only_get_declaration()(to add the enum), neverrun_async. No name-based authority was reintroduced and no alias was added.The tool told the model to call something that does not exist. ADK builds the declaration from
transfer_to_agent's docstring, which through 1.37.0 — the newest release inside our<2pin, checked against the published wheel — advises callers to "use TransferToAgentTool instead of this function directly". That text is written for Python callers but ships to the model, and Gemini 3.1 followed it, emittingTransferToAgentToolfor ADK to reject withTool 'TransferToAgentTool' not found.A narrowly scoped before-model plugin rewrites the description on the prepared request. It acts only when the tool object is exactly
TransferToAgentTooland only while the misleading sentence is present, so it is idempotent and becomes a no-op once the installed ADK ships a corrected docstring (upstream fixed it in 2.x). Onlydescriptionis written — name, parameter schema, required fields and the agent-name enum are preserved, and the upstream__doc__is never mutated.The demo's coordinator prompt gains a matching policy: an explicit bounded request executes or delegates immediately; an open-ended goal, or an explicit request for a plan, is planned and held for confirmation. Planning is stated to be a workflow courtesy — tool permissions remain the authorization boundary.
Verification
Deterministic (offline)
Identity guarantees are asserted alongside the fix: an arbitrary
FunctionToolsubclass, aTransferToAgentToolsubclass, a forged object namedtransfer_to_agent, one named after the class, and one carrying a copied.funcall remain denied.The declaration tests drive a synthetic misleading declaration rather than the installed one, so they never require upstream to stay broken; one integration test accepts either state and asserts the outcome is safe. Re-run against a simulated fixed ADK (patched
transfer_to_agent.__doc__): 23 passed — the correction is a verified no-op for an already-correct description.PTY / wheel
Both smokes complete startup →
/help→/exitwith exit status 0 and no traceback. The wheel acceptance (7 passed) runs through the installedresearch-demoscript, confirmsresearch_demo/agentic_cliresolve inside the venv and outside the repo, and checksdata/benchmarks.csv,skills/report-writer/SKILL.mdandskills/report-writer/assets/report_template.texare present in the installed package.Live (
-m llm, ADK,gemini-2.5-flash— the isolated shipped default)Three independent runs, no retries, no hidden reruns:
gemini-3.1-pro-preview— the model that originally emitted the class name — passes the delegation scenario. It is not the default and nothing was pinned to it.The live suite has exactly three behavioural contracts; the two earlier paid diagnostics were folded into them, so the class-name and unknown-tool assertions cannot pass vacuously — the same test that forbids the class name requires the correct
transfer_to_agentcall.One finding worth recording: an earlier KB scenario failure that looked like an empty completion was, on inspection of the actual event stream, the coordinator correctly refusing — it holds only the KB readers, while
kb_ingest_*belongs toarxiv_specialist. The scenario asked it to call a tool it does not have. That was a test defect, now fixed by asking for the outcome and pointing at the specialist.Post-0.6 follow-up (not in this PR)
Define and document whether
process()propagates backend exceptions or converts them into fatalWorkflowEvent.ERRORevents. A model that invents a tool name currently surfaces as aValueErrorout of the manager stream. The CLI catches it and renders a workflow error, but every non-CLI consumer inherits an undocumented contract. That is a public-contract decision, deliberately out of scope here.Release preparation is intentionally separate
This branch sits directly on
origin/developand contains exactly the twocommits above. The 0.6.0 version bump and CHANGELOG entry are deliberately not
here: they are release preparation, they will be restacked after this merges
(picking up an ADK-delegation entry at that point), and mixing them in would
have made this PR a release PR.
Because of that, the acceptance tests no longer hardcode a release version. The
expected version is derived from
pyproject.tomlwithtomllib, and threesurfaces that can disagree are each checked against it: the wheel filename,
the installed distribution metadata (
importlib.metadata.version), andagentic_cli.__version__. The suite therefore passes ondevelopat 0.5.3today and will validate 0.6.0 automatically when the release change runs through
CI — with no test edit at the bump.
Notes
No CHANGELOG changes — release notes are handled in the separate 0.6.0 documentation pass. No LangGraph files touched. Nothing under
docs/.https://claude.ai/code/session_01BpktWb9vKN5exdT9MbLQYh